Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Working azure download flow #106

Merged
merged 33 commits into from
Dec 16, 2024
Merged

Working azure download flow #106

merged 33 commits into from
Dec 16, 2024

Conversation

zsusswein
Copy link
Collaborator

@zsusswein zsusswein commented Dec 5, 2024 •
edited
Loading

This one turned into a bit of a mad dash to get everything ready for the demo. Everyone was able to run the pipeline locally and in Batch (success!) and I was able to run this branch in a shadow production run today. But now the branch has entirely too much content.

 I'd like to get this PR open and the functionality onto main rather than polishing a long-lived feature branch. If people are on board, that would mean liberally turning feedback into issues and circling back in a series of future PRs.

Thanks for bearing with me on this one folks!

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 5, 2024 •
edited
Loading

Thank you for your contribution @zsusswein 🚀! Your pkgdown-site is ready for download 👉 here 👈!
(The artifact expires on 2024-12-23T23:55:49Z. You can re-generate it by re-running the workflow here.)

@codecov Codecov
Copy link

codecov bot commented Dec 5, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 46.34146% with 44 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
R/azure.R 14.28% 36 Missing ⚠️
R/pipeline.R 80.00% 8 Missing ⚠️
Additional details and impacted files

📢 Thoughts on this report? Let us know!

@kgostic
Copy link
Collaborator

kgostic commented Dec 9, 2024

Great job @zsusswein! Some notes from the demo. Some of these may be separate issues that we deal with later:

  • Roll back .env and config workarounds in demo instructions (after these are fixed)
  • Would be nice to add a mermaid diagram of all the relevant repos, environments, and microservices and how they connect
  • Would be nice to document in each step - from where is this step reading inputs and writing outputs (this would help with troubleshooting)

@zsusswein zsusswein mentioned this pull request Dec 10, 2024
Open
3 tasks
@zsusswein zsusswein marked this pull request as ready for review December 11, 2024 17:24
@zsusswein
Copy link
Collaborator Author

zsusswein commented Dec 11, 2024 •
edited
Loading

FYI @amondal2 I tagged you in case you want to take a look, but the azure stuff is kind of a mess. I'm planning on cleaning it up once this merges and tagging you and Nate to take a look.

natemcintosh
natemcintosh approved these changes Dec 11, 2024
View reviewed changes
Makefile Outdated Show resolved Hide resolved
azure/requirements.txt Outdated Show resolved Hide resolved
azure/job.py Outdated Show resolved Hide resolved
natemcintosh
natemcintosh reviewed Dec 11, 2024
View reviewed changes
Dockerfile Outdated Show resolved Hide resolved
@zsusswein
Copy link
Collaborator Author

Some notes from the demo. Some of these may be separate issues that we deal with later:

Tracking in #116

@zsusswein zsusswein requested a review from athowes December 11, 2024 18:04
natemcintosh
natemcintosh approved these changes Dec 11, 2024
View reviewed changes
Makefile Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
natemcintosh
natemcintosh reviewed Dec 11, 2024
View reviewed changes
azure/job.py Outdated Show resolved Hide resolved
natemcintosh
natemcintosh reviewed Dec 11, 2024
View reviewed changes
Makefile Outdated Show resolved Hide resolved
@zsusswein
Mount dirs in mnt
db06bab
@zsusswein
Put everything in mnt for now
3f5f07f
@zsusswein
Clean up WARNING from undocumented arg
b830dc4
@zsusswein
Ignore Azure stuff in R package
6b6aba6
@zsusswein
Add family for Azure
ec49983
@zsusswein
Pass image name and pool name
4432fc4 
This is a stopgap because we should be able to parse both from the
same variable because everything is named by the branch. But for now,
doing this.
@zsusswein zsusswein enabled auto-merge (squash) December 16, 2024 23:55
@zsusswein
Copy link
Collaborator Author

Merging off of Nate's review per conversation with Katie earlier today.

@zsusswein zsusswein merged commit 2a9798c into main Dec 16, 2024
9 checks passed
@zsusswein zsusswein deleted the edit-azure-flow branch December 16, 2024 23:56
zsusswein added a commit that referenced this pull request Dec 18, 2024
@zsusswein
Re-add pool creation libs to requirements.txt
197992d 
Accidentally removed them in #106
zsusswein added a commit that referenced this pull request Dec 18, 2024
@zsusswein
Re-add pool creation libs to requirements.txt
7aab64e 
Accidentally removed them in #106
zsusswein added a commit that referenced this pull request Dec 18, 2024
@zsusswein
Include missing deps in venv (#130)
3ae30d0 
* Include toml in venv

Should fix pool build, which is currently failing

* Bump NEWS

* Re-add pool creation libs to requirements.txt

Accidentally removed them in #106
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@natemcintosh natemcintosh natemcintosh approved these changes

@kgostic kgostic Awaiting requested review from kgostic

@amondal2 amondal2 Awaiting requested review from amondal2

@kingsleyiyawe kingsleyiyawe Awaiting requested review from kingsleyiyawe

@athowes athowes Awaiting requested review from athowes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zsusswein @kgostic @natemcintosh